Skip to content

Diff-informed queries: phase 3 (non-trivial locations) #19957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 102 commits into
base: main
Choose a base branch
from
Draft

Diff-informed queries: phase 3 (non-trivial locations) #19957

wants to merge 102 commits into from
+11,468 −2,113

Conversation

d10c
Copy link
Contributor

@d10c d10c commented Jul 2, 2025

This PR enables diff-informed mode on queries that select a location other than dataflow source or sink.

I start with automatically generated stubs and then handle each TODO item in its own commit.

@github-actions github-actions bot added C# JS C++ Java Python Go Ruby Rust Pull requests that update Rust code Swift Actions Analysis of GitHub Actions labels Jul 2, 2025
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 2, 2025
View reviewed changes
@d10c d10c force-pushed the d10c/diff-informed-phase-3 branch from ff3a4b9 to 95fe462 Compare July 3, 2025 15:50
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 3, 2025
View reviewed changes
actions/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll
@@ -3,6 +3,7 @@
private import codeql.actions.dataflow.ExternalFlow
private import codeql.actions.security.ArtifactPoisoningQuery
private import codeql.actions.security.UntrustedCheckoutQuery
private import codeql.actions.security.ControlChecks

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
codeql.actions.security.ArtifactPoisoningQuery
Loading
.
@d10c d10c force-pushed the d10c/diff-informed-phase-3 branch 3 times, most recently from aff62c2 to 6d0ae3a Compare July 4, 2025 14:20
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 4, 2025
View reviewed changes
...erimental/query-tests/Security/CWE-208/TimingAttackAgainstHash/TimingAttackAgainstHash.qlref
@@ -0,0 +1 @@
experimental/Security/CWE-208/TimingAttackAgainstHash/TimingAttackAgainstHash.ql

Check warning

Code scanning / CodeQL

Query test without inline test expectations Warning test

Query test does not use inline test expectations.
@d10c d10c force-pushed the d10c/diff-informed-phase-3 branch 2 times, most recently from c871f5e to 276c7f0 Compare July 7, 2025 09:42
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 7, 2025
View reviewed changes
go/ql/lib/semmle/go/security/HardcodedCredentials.qll
Comment on lines +39 to +45
result = source.getLocation()
or
exists(DataFlow::Node node | result = node.getLocation() |
sensitiveAssignment(node, _, _)
or
hardcodedPrivateKey(node, _)
)

Check warning

Code scanning / CodeQL

Var only used in one side of disjunct. Warning

The
variable source
Loading
is only used in one side of disjunct.
go/ql/lib/semmle/go/security/HardcodedCredentials.qll
Comment on lines +49 to +55
result = sink.getLocation()
or
exists(DataFlow::Node node | result = node.getLocation() |
sensitiveAssignment(_, node, _)
or
hardcodedPrivateKey(node, _)
)

Check warning

Code scanning / CodeQL

Var only used in one side of disjunct. Warning

The
variable sink
Loading
is only used in one side of disjunct.
@d10c d10c force-pushed the d10c/diff-informed-phase-3 branch 2 times, most recently from cb2db2f to c70036d Compare July 8, 2025 15:30
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 8, 2025
View reviewed changes
java/ql/lib/semmle/code/java/security/SqlConcatenatedQuery.qll
Comment on lines +38 to +43
exists(QueryInjectionSink query, Expr uncontrolled |
result = [query.getLocation(), uncontrolled.getLocation()] and
builtFromUncontrolledConcat(query.asExpr(), uncontrolled)
)
or
result = sink.getLocation()

Check warning

Code scanning / CodeQL

Var only used in one side of disjunct. Warning

The
variable sink
Loading
is only used in one side of disjunct.
java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll
Comment on lines +152 to +154
result = source.getLocation()
or
result = any(MethodCallInsecureFileCreation m).getLocation()

Check warning

Code scanning / CodeQL

Var only used in one side of disjunct. Warning

The
variable source
Loading
is only used in one side of disjunct.
java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll
Comment on lines +24 to +26
unsafeTrust instanceof RabbitMQEnableHostnameVerificationNotSet
or
sink.asExpr() = unsafeTrust

Check warning

Code scanning / CodeQL

Var only used in one side of disjunct. Warning

The
variable sink
Loading
is only used in one side of disjunct.
@d10c d10c force-pushed the d10c/diff-informed-phase-3 branch from c70036d to 08c4cc2 Compare July 9, 2025 16:47
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 9, 2025
View reviewed changes
cpp/ql/src/Security/CWE/CWE-732/UnsafeDaclSecurityDescriptor.ql
Comment on lines +47 to +50
call.getArgument(1).getValue().toInt() != 0 and
call.getArgument(2) instanceof NullValue
or
sink.asExpr() = call.getArgument(2)

Check warning

Code scanning / CodeQL

Var only used in one side of disjunct. Warning

The
variable sink
Loading
is only used in one side of disjunct.
@d10c d10c force-pushed the d10c/diff-informed-phase-3 branch from 08c4cc2 to 857b583 Compare July 10, 2025 17:04
d10c added 28 commits July 11, 2025 14:47
@d10c
Java: SqlConcatenated
1072b58
@d10c
Java: SqlInjection
69f8688
@d10c
Java: TempDirLocalInformationDisclosure
51bcd34
@d10c
Java: TrustBoundaryViolations (convert test to qlref)
b0a314c
@d10c
Java: UnsafeCertTrust (+ convert test to qlref)
900b4a3
@d10c
Java: AndroidWebViewSettingsAllowsContentAccess
633c6ca
@d10c
JS: patch-generated stubs
02693a6
@d10c
JS: IndirectCommandInjection
b7f4255
@d10c
JS: NosqlInjection, SqlInjection
031e4c8
@d10c
JS: ShellCommandInjection
9fc93e0
@d10c
JS: EnvValueAndKeyInjection
5112eb3
@d10c
JS: decodeJwtWithoutVerification
427a852
@d10c
Python: patch-generated stubs
e0cd030
@d10c
Python: LdapInjection
58e9e4a
@d10c
Python: WeakSensitiveDatHashing
86671a9
@d10c
Python: PossibleTimingAttackAgainstHash (+ selecting source node inst…
fbdf962 
…ead of string)
@d10c
Python: TimingAttackAgainstHash (+ new test)
195b013 
That said, the test is useless; to get a result, I need a comparison against remote input.
@d10c
Ruby: patch-generated stubs
83586ff
@d10c
Ruby: MissingFullAnchor
5a8c2c9
@d10c
Ruby: WeakSensitiveDataHashing
5a130fe
@d10c
Ruby: WeakFilePermissions
525ca3b
@d10c
Rust: patch-generated stubs
d39010b
@d10c
Rust: AccessAfterLifetime
67d615a
@d10c
Swift: patch-generated stubs
0d20533
@d10c
Swift: CleartextStorageDatabase
9cb57cc
@d10c
Swift: CleartextStoragePreferences
02d7fea
@d10c
Swift: UnsafeWebViewFetch
057e266
@d10c
Swift: InsecureTLSQuery
3e3e856
@d10c d10c force-pushed the d10c/diff-informed-phase-3 branch from 857b583 to 3e3e856 Compare July 11, 2025 12:49
@d10c
Copy link
Contributor Author

d10c commented Jul 15, 2025

DCA results: some slowdowns on Python, but they don't seem to be related to these changes. Java had some timeout-related failures, so I'll restart that. Overall, no negative performance impact on empty-diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Actions Analysis of GitHub Actions C# C++ Go Java JS Python Ruby Rust Pull requests that update Rust code Swift
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@d10c